-
Notifications
You must be signed in to change notification settings - Fork 4.2k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
LSTM models from 1.x will be incompatible with MLA 2.x #5254
Conversation
@@ -79,6 +79,8 @@ public BuiltInSensorType GetBuiltInSensorType() | |||
public class ParameterLoaderTest | |||
{ | |||
const string k_discrete_ONNX_v2 = "Packages/com.unity.ml-agents/Tests/Editor/TestModels/discrete_rank2_vector_v2_0.onnx"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we rename the old model file to something like "obsolete_lstm_v1.onnx"? And rename the constants and NNModel variables accordingly?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I renamed to discrete1vis0vec_2_3action_obsolete_recurr_v1_0.onnx. I remade this model with version 15 since it seems this model was actually not a recurrent model.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Please update the changelog and migration guide.
Discussed offline, but some code that got added in #5236 can be removed, and probably BarracudaMemoryOutputApplier
and BarracudaRecurrentInputGenerator
too.
… were removed. All code that checked for (memories + v1.X) have been removed since these will no longer be supported
com.unity.ml-agents/CHANGELOG.md
Outdated
@@ -33,6 +33,13 @@ different sizes using the same model. For a summary of the interface changes, pl | |||
- `GridSensor` has been refactored and moved to main package, with changes to both sensor interfaces and behaviors. | |||
Exsisting GridSensor created by extension package will not work in newer version. Previously trained models will | |||
need to be retrained. Please see the Migration Guide for more details. (#5256) | |||
- Models trained with 1.x versions of ML-Agents will no longer work at inference if they were trained using recurrent neural networks (#5254) | |||
|
|||
#### ml-agents / ml-agents-envs / gym-unity (Python) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this intentional? It was in 1.9.1, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No it is not. I think I made a mistake when resolving conflicts in the last commit.
Proposed change(s)
Making 1.x models that contain LSTM not work with MLA 2.0.
This is to prepare for barracuda 2.0 that will break compatibility for LSTM.
Useful links (Github issues, JIRA tickets, ML-Agents forum threads etc.)
Types of change(s)
Checklist
Other comments